Skip to content

micro: Handle +/text search text from args#3767

Merged
JoeKar merged 1 commit intomicro-editor:masterfrom
luca020400:luca/plusslash
Sep 5, 2025
Merged

micro: Handle +/text search text from args#3767
JoeKar merged 1 commit intomicro-editor:masterfrom
luca020400:luca/plusslash

Conversation

@luca020400
Copy link
Contributor

This is a feature found in vim and commonly used
by Linux kernel test robots to give context about
warnings and/or failures.

e.g. vim +/imem_size +623 drivers/net/ipa/ipa_mem.c

The order in which the commands appear in the args
determines the order in which the "goto line:column"
and search text will be executed.

Copy link
Contributor

@Andriamanitra Andriamanitra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think NewBufferFromFileWithCommand is a weird API to have because it's coupled to how the CLI works. It also requires changing all the call sites. Do you think it would be feasible to keep NewBufferFromFileAtLoc/NewBuffer as-is and handle moving the cursor to a match separately?

@luca020400
Copy link
Contributor Author

That's possible, but it's not that different from the current API as a whole, it would be equivalent to simply adding searchText parameter (as start location was only used for CLI already)

And if I split open & match i'd be weird to handle the save cursor option and furthermore the onBufferOpen would run before finding matches, and now you'd have start position (either from +lin:col for filename), onBufferOpen and then match, it would be very confusing to me...

Feel free to propose something, but this seemed the "sanest" model to implement to me

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 8, 2025

I think NewBufferFromFileWithCommand is a weird API to have

That was my first though too, but after a closer look, it looks like quite a sane generalization of AtLoc. Also:

it's coupled to how the CLI works

Potentially we might use this concept of "command" not just in the CLI but also in some other cases at runtime, like it is used in vim (and vi, ed, sed...).

@luca020400
Copy link
Contributor Author

Ping?

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 23, 2025

@JoeKar what do you think about merging this?

@JoeKar
Copy link
Member

JoeKar commented Jun 24, 2025

@JoeKar what do you think about merging this?

No bad idea and...

Potentially we might use this concept of "command" not just in the CLI [...]

...we're able to extend the command easily.

What I'm struggling with is the fact, that NewBufferFromFileAtLoc() is gone without any simple wrapper to NewBufferFromFileWithCommand(). This will affect plugins currently using this method.
Furthermore we shouldn't introduce any further regexp.MustCompile()'s, especially when user input is parsed. I remember here #3700, which caused the panic looking like a crash of micro's code.

@dmaluka
Copy link
Collaborator

dmaluka commented Jun 24, 2025

This will affect plugins currently using this method.

Are they? I don't think we export it to lua.

Furthermore we shouldn't introduce any further regexp.MustCompile()'s, especially when user input is parsed.

But here we don't pass any user input (or any other non-constant input) to regexp.MustCompile(), we only pass constant strings? They either always compile, or never compile?

@luca020400
Copy link
Contributor Author

@JoeKar what do you think about merging this?

No bad idea and...

Potentially we might use this concept of "command" not just in the CLI [...]

...we're able to extend the command easily.

What I'm struggling with is the fact, that NewBufferFromFileAtLoc() is gone without any simple wrapper to NewBufferFromFileWithCommand(). This will affect plugins currently using this method.

I admit I'm not particularly aware of how plugins work, but I don't expect go functions to be directly callable from there? That's why I removed it as I assumed it wasn't exported.

Furthermore we shouldn't introduce any further regexp.MustCompile()'s, especially when user input is parsed. I remember here #3700, which caused the panic looking like a crash of micro's code.

But isn't that more of an architectural issue?
If for the search functionality you enforce the regex to be valid that means even the current find functionality is broken, I simply use that infrastructure.

@JoeKar
Copy link
Member

JoeKar commented Jun 25, 2025

This will affect plugins currently using this method.

Are they? I don't think we export it to lua.

You're right. It can't be accessed.

Furthermore we shouldn't introduce any further regexp.MustCompile()'s, especially when user input is parsed.

But here we don't pass any user input (or any other non-constant input) to regexp.MustCompile(), we only pass constant strings? They either always compile, or never compile?

You're right again. Here we only search for the search string within the arguments. The user string will cause problems at the same location as in #3700 (with +/\\Q).

But isn't that more of an architectural issue?

At least a bad design decision, in case the "wrong" regexp interface is used for arbitrary user input.

that means even the current find functionality is broken

More or less yes. Some corner cases will actually lead to a crash (see linked issue).

@luca020400
Copy link
Contributor Author

Can this be merged?

@JoeKar
Copy link
Member

JoeKar commented Sep 3, 2025

We can merge it in case @dmaluka doesn't disagree.

@dmaluka
Copy link
Collaborator

dmaluka commented Sep 3, 2025

LGTM (I think I already approved this PR a while ago), except the following (echoing the discussion and outcome of #3812):

  1. This?
--- a/cmd/micro/micro.go
+++ b/cmd/micro/micro.go
@@ -48,7 +48,7 @@ var (
 func InitFlags() {
 	// Note: keep this in sync with the man page in assets/packaging/micro.1
 	flag.Usage = func() {
-		fmt.Println("Usage: micro [OPTION]... [FILE]... [+LINE[:COL]]")
+		fmt.Println("Usage: micro [OPTION]... [FILE]... [+LINE[:COL]] [+/REGEX]")
 		fmt.Println("       micro [OPTION]... [FILE[:LINE[:COL]]]...  (only if the `parsecursor` option is enabled)")
 		fmt.Println("-clean")
 		fmt.Println("    \tClean the configuration directory and exit")
  1. Keep the man page in sync, i.e. this?
diff --git a/assets/packaging/micro.1 b/assets/packaging/micro.1
index dbd2cc80..909bbde6 100644
--- a/assets/packaging/micro.1
+++ b/assets/packaging/micro.1
@@ -1,11 +1,12 @@
-.TH micro 1 "2025-08-16"
+.TH micro 1 "2025-09-03"
 .SH NAME
 micro \- A modern and intuitive terminal-based text editor
 .SH SYNOPSIS
 .B micro
 .RI [ OPTION ]...\&
 .RI [ FILE ]...\&
-.RI [+ LINE [: COL ]]
+.RI [+ LINE [: COL ]]\&
+.RI [+/ REGEX ]
 .br
 .B micro
 .RI [ OPTION ]...\&
@@ -40,6 +41,11 @@ Specify a custom location for the configuration directory
 Specify a line and column to start the cursor at when opening a buffer
 .RE
 .PP
+.RI +/ REGEX
+.RS 4
+Specify a regex to search for when opening a buffer
+.RE
+.PP
 .B \-options
 .RS 4
 Show all options help and exit

This is a feature found in vim and commonly used
by Linux kernel test robots to give context about
warnings and/or failures.

e.g. vim +/imem_size +623 drivers/net/ipa/ipa_mem.c

The order in which the commands appear in the args
determines in which order the "goto line:column"
and search will be executed.
@luca020400
Copy link
Contributor Author

Done, I hope the man changes are good, I don't know the syntax at all

@JoeKar JoeKar merged commit e9f241a into micro-editor:master Sep 5, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants